chore: document and push changes in ci on pull requests#460
Conversation
toph-allen
changed the title
toph-allen
changed the title
karawoo
left a comment
karawoo
left a comment
There was a problem hiding this comment.
I can't really picture a scenario when we wouldn't want the documentation kept up to date, so doing it automatically seems worth it to me.
That said, I am not sure how this will behave on PRs from forks, which IIUC get a read-only github token by default. So I might leave the /document version in place for now but add the automatic one, then after merge we could try both on a sample PR from a fork and see if they both work or not. It's possible that /document would work when the automatic one doesn't because the triggering event is a comment (which has to be from a repo member) and not a PR.
@karawoo Agreed with those concerns. I restored the So it sounds like we should commit this then open a PR from a fork… from an account that isn't a member of this repo? Can do. |
Yeah I think so. you could temporarily remove me from the repo and then add me back after the test if you want. |
There was a problem hiding this comment.
just curious where these .Rd file changes came from? It's not totally obvious to me what these changes do
There was a problem hiding this comment.
Aha, newer versions of roxygen2::roxygenize() make these changes. I'll make sure the renv.lock is updated so we get consistency between local and CI docs.
There was a problem hiding this comment.
oh interesting, I didn't realize we had an renv.lock in this repo because it's not automatically activated
There was a problem hiding this comment.
I'm actually currently developing on an older version of R than I was when the renv.lock was last updated (for Connect Gallery version compat with 4.3). I'm fine holding that for a different PR if you want. Either way — these .Rd changes are fine and normal.
There was a problem hiding this comment.
I'd actually be fine removing it entirely!
There was a problem hiding this comment.
yeah, I don't feel strongly either way, was just confused because when I run roxygenize locally on main it didn't generate the same changes. but I must be on a different version.
There was a problem hiding this comment.
Looks like it was literally in the latest version! https://roxygen2.r-lib.org/news/index.html?utm_source=chatgpt.com#roxygen2-733
2 participants
Intent
Following discussion on #295, I whipped up a PR to automatically run
roxygen2::roxygenize()in CI on pull requests and push the result back to the branch.Doing a bit of digging, though, I discovered that the repo was already set up to do this if someone comments "/document" on a pull requests. @karawoo Do you think that's good enough, or do you think an auto-document workflow is better?
Fixes #295
Checklist
NEWS.md(referencing the connected issue if necessary)?devtools::document()?